Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ githubrepo: Allow providing an already authenticated transport #1644

Merged
merged 1 commit into from
Feb 16, 2022

Conversation

jeffmendoza
Copy link
Member

Way back in 2.0 the constructor took a github.Client directly, which is what Allstar instantiated to then run scorecard checks. With the current code, there is no way to override the built-in roundtripper.NewTransport which uses its own methods to authenticate.

To allow Allstar to use the current code, we need to create this client and provide a pre-authenticated transport (or http client, or github client).

cc @justaugustus

@justaugustus
Copy link
Member

@jeffmendoza -- Hehe, I just opened a branch for this! Mind if I push a few things on top of this PR?

@github-actions
Copy link

Integration tests success for
[0d49b00]
(https://github.com/ossf/scorecard/actions/runs/1850042265)

@codecov-commenter
Copy link

Codecov Report

Merging #1644 (0d49b00) into main (cda7a1b) will increase coverage by 2.83%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1644      +/-   ##
==========================================
+ Coverage   56.12%   58.95%   +2.83%     
==========================================
  Files          72       72              
  Lines        6493     6495       +2     
==========================================
+ Hits         3644     3829     +185     
+ Misses       2604     2415     -189     
- Partials      245      251       +6     

@azeemshaikh38
Copy link
Contributor

Will we need to make a Scorecard release for AllStar to be able to consume it? If so, please do chime in #1621. We can wait for this PR to go in before cutting the release.

@jeffmendoza jeffmendoza changed the title Add second constructor to allow providing an already authenticated transport ✨ Add second constructor to allow providing an already authenticated transport Feb 16, 2022
@justaugustus justaugustus changed the title ✨ Add second constructor to allow providing an already authenticated transport ✨ githubrepo: Allow providing an already authenticated transport Feb 16, 2022
@justaugustus justaugustus merged commit ba503c3 into ossf:main Feb 16, 2022
@justaugustus
Copy link
Member

@jeffmendoza -- Hehe, I just opened a branch for this! Mind if I push a few things on top of this PR?

Will we need to make a Scorecard release for AllStar to be able to consume it? If so, please do chime in #1621. We can wait for this PR to go in before cutting the release.

Merged. Will push any updates as a follow-up! Thanks Jeff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants